-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RF] Enable streaming of RooLagrangianMorphFunc and other improvements #11023
[RF] Enable streaming of RooLagrangianMorphFunc and other improvements #11023
Conversation
This commit removes the `_curNormSet` and `_ownParameters` member variables from the RooLagrangianMorphFunc. The `_ownParameters` was simply unused, and the `_curNormSet` was redundant because there is already `RooAbsReal::_lastNSet` that also points to the current normalization set. Now, the `RooLagrangianMorphFunc::getValV()` can also be removed because the sole purpose was to set `_curNormSet`, and also the private `RooLagrangianMorphFunc::getCache()` interface can be changed to take no normSet parameter (as it was unused anyway). The `RooLagrangianMorphFunc::_cacheMgr` declaration is also moved to the bottom of the file together with the other member variables.
This is to enable IO for the RooLagrangianMorphFunc.
The title for the x-axis of the first plot should be `"p_{T}^{V}"` and not `"c_{Hq^{(3)}}"`. Also, the title for the observable is changed to `"p_{T}^{V}"` to get the right axis label for the x-axes of the plots.
Starting build on |
Starting build on |
The RooLagrangianMorphFunc was a desaster concerning memory safety. This commit fixes almost all of the memory leaks that were easy to spot because of the usage of `new`. One leak that still remains are the `RooDataHist`s that underlies the `RooHistFunc`s. But to fix that, it would be important to first come up with an elegant way to have the RooHistFunc own their underlying RooDataHist.
c3779b0
to
4a96120
Compare
Starting build on |
Hi @guitargeek thanks a lot for fixing this important issue ! I tested locally and the serialisation is fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thank you Jonas for fixing this !
Build failed on mac1015/cxx17. Failing tests: |
This PR enables the streaming of the RooLagrangianMorph by creating dictionaries also for
RooLagrangianMorph::Config
.Furthermore, some improvements are made to the RooLagrangianMorphFunc and its tutorials in separate commits.
Tons of memory leaks are fixed in the
RooLagrangianMorphFunc
as well.